Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add rebase how-to steps and one per branch suggestion #97

Merged
merged 9 commits into from
Dec 20, 2023

Conversation

avallecam
Copy link
Member

@avallecam avallecam commented Dec 19, 2023

This updates the steps for:

  • how to rebase in local and remote repositories.
  • try to keep one contributor per feature branch.

These modifications will propagate to all workbench-based repositories.

Copy link

github-actions bot commented Dec 19, 2023

Thank you!

Thank you for your pull request 😃

🤖 This automated message can help you check the rendered files in your submission for clarity. If you have any questions, please feel free to open an issue in {sandpaper}.

If you have files that automatically render output (e.g. R Markdown), then you should check for the following:

  • 🎯 correct output
  • 🖼️ correct figures
  • ❓ new warnings
  • ‼️ new errors

Rendered Changes

🔍 Inspect the changes: https://github.com/epiverse-trace/tutorials/compare/md-outputs..md-outputs-PR-97

The following changes were observed in the rendered markdown documents:


What does this mean?

If you have source files that require output and figures to be generated (e.g. R Markdown), then it is important to make sure the generated figures and output are reproducible.

This output provides a way for you to inspect the output in a diff-friendly manner so that it's easy to see the changes that occur due to new software versions or randomisation.

⏱️ Updated at 2023-12-20 11:55:54 +0000

@avallecam avallecam requested a review from Bisaloo December 19, 2023 17:15
github-actions bot pushed a commit that referenced this pull request Dec 19, 2023
github-actions bot pushed a commit that referenced this pull request Dec 19, 2023
github-actions bot pushed a commit that referenced this pull request Dec 19, 2023
github-actions bot pushed a commit that referenced this pull request Dec 19, 2023
github-actions bot pushed a commit that referenced this pull request Dec 19, 2023
github-actions bot pushed a commit that referenced this pull request Dec 19, 2023
@avallecam
Copy link
Member Author

avallecam commented Dec 19, 2023

@Bisaloo after finishing with the edits in the PR, in parallel I merged #95.

This made visible the Update with Rebase option, so I did it, labelling the last commit as "force-pushed" as expected.

Then, in my local repo, I did git pull generating a local merge (not wanted). Please, look at the output messages, commit history, and screenshot from RStudio History pane:

~/tutorials (update-howto-rebase)
$ git pull
remote: Enumerating objects: 48, done.
remote: Counting objects: 100% (48/48), done.
remote: Compressing objects: 100% (15/15), done.
remote: Total 48 (delta 38), reused 36 (delta 33), pack-reused 0
Unpacking objects: 100% (48/48), 14.52 KiB | 68.00 KiB/s, done.
From https://github.com/epiverse-trace/tutorials
 + c3aef90...9df13a4 update-howto-rebase -> origin/update-howto-rebase  (forced update)
   173598b..57d6a3d  gh-pages            -> origin/gh-pages
   46ef385..bc71b50  main                -> origin/main
   b189050..2137f56  md-outputs          -> origin/md-outputs
 + f34b77a...1ff0593 md-outputs-PR-97    -> origin/md-outputs-PR-97  (forced update)
Merge made by the 'ort' strategy.
 config.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

~/tutorials (update-howto-rebase)
$ git log
commit 541c73bbf3eae21850ac64bf02b6e817e2b4add9 (HEAD -> update-howto-rebase)
Merge: c3aef90 9df13a4
Author: Andree  Valle Campos <[email protected]>
Date:   Tue Dec 19 18:10:16 2023 +0000

    Merge branch 'update-howto-rebase' of https://github.com/epiverse-trace/tutorials
into update-howto-rebase

commit 9df13a4ced0584c166f2603d2d8a8d19bd523a47 (origin/update-howto-rebase)
Author: Andree  Valle Campos <[email protected]>
Date:   Tue Dec 19 18:00:01 2023 +0000

    add warning to do rebase only for one contributor per branch

image

At this moment I stopped and avoided pushing this content to GitHub. I imagine this would add a merge commit in the PR history and disallow the "rebase and merge" option.

@Bisaloo
Copy link
Member

Bisaloo commented Dec 19, 2023

To avoid this, you need to run git pull --rebase instead of a simple git pull.

You can also change your global settings via git config --global pull.rebase true so that git pull means git pull --rebase by default.

@avallecam
Copy link
Member Author

To avoid this, you need to run git pull --rebase instead of a simple git pull.

That step was the missing bit in the process. Thanks!

CONTRIBUTING.md Outdated
Comment on lines 123 to 124
- Keep your feature branch up to date with `main`.
- This encourage to keep the history as linear as possible, prioritizing actions like ["rebase and merge"](https://epiverse-trace.github.io/blueprints/git-branching-merging.html#merging-pull-requests-merge-commits-vs-squash-and-merge-vs-rebase-and-merge) to accept the PR.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Keeping in sync with main may not be necessary in most cases. git/GitHub is very good at identifying potential sources of conflicts and unless you get a message saying you have conflicts that you need to solve, rebasing your branch or merging the main branch into feature is extra work for (almost) no gain.

The linearity of this history is unrelated to our well you keep in sync with main. It is ensured by the absence of merge commits in the history.

Copy link
Member

@Bisaloo Bisaloo Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically, in the present PR, it was probably not necessary to rebase on main, even after your latest commits. You can do it, but it's an extra source of potential mistakes.

Copy link
Member Author

@avallecam avallecam Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From all these experiences, I confirm that this added unintentional extra mistakes.

So, I think we could change the "Keep your feature branch..." line as an optional action:

If you need to get the latest commits in the main branch to use them in your development, we recommend rebasing your feature branch. This will keep to commit history linear.

  • In the local repository...
    1. switch...
    2. pull...
      ...
  • In the remote...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sounds perfect!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added in 0ac8b03

github-actions bot pushed a commit that referenced this pull request Dec 19, 2023
github-actions bot pushed a commit that referenced this pull request Dec 20, 2023
github-actions bot pushed a commit that referenced this pull request Dec 20, 2023
github-actions bot pushed a commit that referenced this pull request Dec 20, 2023
@avallecam avallecam requested a review from Bisaloo December 20, 2023 11:57
@avallecam avallecam merged commit ba64a2b into main Dec 20, 2023
3 checks passed
@avallecam avallecam deleted the update-howto-rebase branch December 20, 2023 12:40
avallecam added a commit that referenced this pull request Dec 20, 2023
amanda-minter pushed a commit to amanda-minter/tutorials-late that referenced this pull request Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants